fix: sync MCP sessions to Redis for multi-replica deployments - #3491

backend/app/src/test/kotlin/io/tolgee/mcp/McpRedisSessionRecoveryTest.kt (1)

65-94: Good integration test covering the core recovery flow.

The test cleanly validates the end-to-end scenario: session creation → Redis persistence → local eviction → recovery → successful tool call. The use of reflection to access the sessions map mirrors the filter's approach, keeping the test realistic.

One minor gap: the test doesn't assert that the recovered session's capabilities/client info match the original. If the serialization/deserialization round-trip has a bug, the session would exist in the map but with null/wrong metadata.

Optional: verify session data fidelity

// After step 5, verify the recovered session has correct data
val recoveredSession = sessionsMap[sessionId]!!
// Use the same reflection approach as the filter to check clientInfo is non-null
val clientInfoField = McpStreamableServerSession::class.java.getDeclaredField("clientInfo")
clientInfoField.isAccessible = true
val clientInfo = (clientInfoField.get(recoveredSession) as? java.util.concurrent.atomic.AtomicReference<*>)?.get()
assertThat(clientInfo).isNotNull

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/app/src/test/kotlin/io/tolgee/mcp/McpRedisSessionRecoveryTest.kt`
around lines 65 - 94, Update the test method session is recovered from Redis
after local eviction to also verify the recovered session's metadata: after step
5 retrieve the recoveredSession from sessionsMap using sessionId, use reflection
on McpStreamableServerSession to access the private clientInfo field (make it
accessible), unwrap the AtomicReference to get the underlying clientInfo object
and assert it is not null so the serialization/deserialization preserved client
info; reference symbols: McpRedisSessionRecoveryTest, sessionsMap, sessionId,
McpStreamableServerSession, clientInfo.

backend/app/src/main/kotlin/io/tolgee/mcp/McpSessionRedisFilter.kt (2)

87-118: Recovered session lacks SSE transport — subsequent streaming may fail.

The McpStreamableServerSession constructed here has clientCapabilities, clientInfo, requestTimeout, and handlers — but it does not restore the session's SSE transport or any active subscriptions. If the MCP protocol requires server-to-client push (e.g., notifications, progress updates) on a recovered session, those will silently fail because no transport is connected.

This is likely acceptable for request/response tool calls (which is what the test covers), but worth documenting as a known limitation so that future developers don't assume full session fidelity after recovery.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/app/src/main/kotlin/io/tolgee/mcp/McpSessionRedisFilter.kt` around
lines 87 - 118, The recovered session created in recoverSessionFromRedis using
McpStreamableServerSession lacks any SSE transport or subscription state, so
server-to-client streaming/notifications will silently fail; update
recoverSessionFromRedis to restore the SSE transport and any active
subscriptions from McpSessionData if those fields exist (rehydrate transport and
subscription lists into the new McpStreamableServerSession and reattach into
sessionsMap), and if no transport/subscriptions are stored, explicitly mark the
session as non-streamable and log a clear warning so callers know streaming is
unavailable; reference McpStreamableServerSession, McpSessionData,
recoverSessionFromRedis and sessionsMap when making the changes.

---

226-230: Consider making REDIS_KEY_PREFIX configurable or include a version discriminator.

If the SDK constructor signature changes across versions, stale Redis entries written by an older version could be deserialized into an incompatible session on a newer replica. Including a schema version in the key prefix (e.g., mcp_session:v1:) would let you invalidate old entries on upgrade without manual Redis flushes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/app/src/main/kotlin/io/tolgee/mcp/McpSessionRedisFilter.kt` around
lines 226 - 230, REDIS_KEY_PREFIX is currently hardcoded in the
McpSessionRedisFilter companion object; make the Redis key prefix configurable
or include a schema/version discriminator (e.g., "mcp_session:v1:") so old
entries can be invalidated on upgrades. Change REDIS_KEY_PREFIX to be
injected/read from configuration (or build it from an application property like
mcp.session.redisPrefix with default "mcp_session:v1:") and update all places
that reference REDIS_KEY_PREFIX in McpSessionRedisFilter to use the new
configurable value; ensure the default includes the version token and document
the property for future version bumps.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

backend/app/src/main/kotlin/io/tolgee/mcp/McpSessionRedisFilter.kt (1)

72-74: ⚠️ Potential issue | 🟡 Minor

Avoid non-atomic recover/insert for the same session ID.

Line 72 + Line 114 allow concurrent requests to reconstruct and overwrite the same session. Use atomic insertion (putIfAbsent/computeIfAbsent) to prevent duplicate construction and overwrite races.

💡 Suggested concurrency-safe adjustment

-      sessionsMap[sessionId] = session
+      sessionsMap.putIfAbsent(sessionId, session)

-    if (sessionId != null && !sessionsMap.containsKey(sessionId)) {
+    if (sessionId != null && !sessionsMap.containsKey(sessionId)) {
       recoverSessionFromRedis(sessionId)
     }

(Keep the pre-check, but ensure final insertion is atomic.)

Also applies to: 114-114

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/app/src/main/kotlin/io/tolgee/mcp/McpSessionRedisFilter.kt` around
lines 72 - 74, The current non-atomic pre-check + recover flow can lead to
duplicate/overwriting sessions for the same sessionId; update the insertion into
sessionsMap to be atomic (use sessionsMap.putIfAbsent(sessionId, session) or
sessionsMap.computeIfAbsent(sessionId, id -> recoveredSession)) in the
recoverSessionFromRedis flow so concurrent threads cannot reconstruct and
overwrite the same session; keep the existing pre-check (if (sessionId != null
&& !sessionsMap.containsKey(sessionId)) ...) but ensure the final store uses an
atomic method in McpSessionRedisFilter where sessionId, sessionsMap and
recoverSessionFromRedis are used.

backend/app/src/test/kotlin/io/tolgee/mcp/McpRedisSessionRecoveryTest.kt (1)

60-63: ⚠️ Potential issue | 🟡 Minor

Add per-test cleanup for created TestData.

This test creates persisted data in @BeforeEach but does not clean it in @AfterEach. Please add cleanup for data.testData to keep isolation between test methods and avoid data accumulation.

As per coding guidelines "Use TestData classes for test setup following the pattern: create TestData instance, save with testDataService, and clean in @AfterEach".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/app/src/test/kotlin/io/tolgee/mcp/McpRedisSessionRecoveryTest.kt`
around lines 60 - 63, The setup() method calls createTestDataWithPat() and
stores the result in data but there's no per-test cleanup; add an `@AfterEach`
method that calls testDataService.delete(data.testData) (or the TestData cleanup
helper used elsewhere) to remove the persisted TestData after each test,
ensuring you reference the same data variable produced by
createTestDataWithPat() and import/annotate the cleanup method with `@AfterEach`
so test isolation is preserved.

backend/app/src/main/kotlin/io/tolgee/mcp/McpSessionRedisFilter.kt (1)

218-220: Use case-insensitive header name matching in wrapper.

Line 218 and Line 228 compare header names case-sensitively. Use equals(..., ignoreCase = true) so alternate casing still captures Mcp-Session-Id.

Also applies to: 228-230

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/app/src/main/kotlin/io/tolgee/mcp/McpSessionRedisFilter.kt` around
lines 218 - 220, In the header-wrapping logic inside McpSessionRedisFilter (when
comparing incoming header names to MCP_SESSION_ID_HEADER) change the
case-sensitive comparisons that use == to use equals(..., ignoreCase = true) so
alternate header casing (e.g., "Mcp-Session-Id") is captured; update both places
where name == MCP_SESSION_ID_HEADER (the block that sets capturedSessionId and
the later comparison around lines referencing capturedSessionId) to use
MCP_SESSION_ID_HEADER.equals(name, ignoreCase = true).

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

backend/app/src/main/kotlin/io/tolgee/mcp/McpSessionRedisFilter.kt (1)

71-74: ⚠️ Potential issue | 🟠 Major

Guard pre-handle local map lookup from reflective failures.

Line 72 can throw if reflective map loading breaks (SDK internals change), and this path is currently not protected, so request handling can fail before reaching fallback behavior.

💡 Suggested hardening

-    if (sessionId != null && !sessionsMap.containsKey(sessionId)) {
-      recoverSessionFromRedis(sessionId)
-    }
+    if (sessionId != null) {
+      val missingLocally =
+        runCatching { !sessionsMap.containsKey(sessionId) }
+          .onFailure { log.warn("Failed to access MCP local sessions map; skipping Redis recovery", it) }
+          .getOrDefault(false)
+      if (missingLocally) {
+        recoverSessionFromRedis(sessionId)
+      }
+    }

#!/bin/bash
set -euo pipefail

FILE="$(fd "McpSessionRedisFilter.kt" --type f | head -1)"
echo "Inspecting: $FILE"
echo
echo "=== Pre-handle block ==="
sed -n '68,78p' "$FILE"
echo
echo "=== sessionsMap access context ==="
rg -n 'sessionsMap\.containsKey\(|recoverSessionFromRedis\(' "$FILE" -C2

Expected result: the pre-handle containsKey call is shown directly in the condition, without local error-guarding.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/app/src/main/kotlin/io/tolgee/mcp/McpSessionRedisFilter.kt` around
lines 71 - 74, The containsKey check on sessionsMap in the pre-handle block can
throw if reflective map loading breaks; wrap the
sessionsMap.containsKey(sessionId) call in a protective try-catch (or safe
helper) inside the pre-handle so any Throwable from the reflective lookup is
caught and treated as “not present”, then call
recoverSessionFromRedis(sessionId) as the fallback; update the logic around the
existing sessionsMap / recoverSessionFromRedis usage so exceptions from
containsKey do not propagate out of the pre-handle.

backend/app/src/test/kotlin/io/tolgee/mcp/McpRedisSessionRecoveryTest.kt (1)

84-84: Make session selection deterministic in the test.

Using first() on ConcurrentHashMap keys can become order-dependent if extra sessions exist, which can make this test flaky.

✅ Suggested tweak

-    val sessionId = sessionsMap.keys().toList().first()
+    val sessionId =
+      sessionsMap.keys().singleOrNull()
+        ?: error("Expected exactly one MCP session in map, got ${sessionsMap.keys()}")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/app/src/test/kotlin/io/tolgee/mcp/McpRedisSessionRecoveryTest.kt` at
line 84, The test McpRedisSessionRecoveryTest uses
sessionsMap.keys().toList().first() which is non-deterministic; make selection
deterministic by ordering the keys before picking one (e.g., use
sorted()/minOrNull() on sessionsMap.keys()) so sessionId is consistently the
same across runs and the test stops being flaky.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro